-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stop excluding observerless queries from refetchQueries
selection
#8825
Conversation
if (fetchPolicy === "standby" || !oq.hasObservers()) { | ||
// Skip inactive queries unless include === "all". | ||
if ( | ||
fetchPolicy === "standby" || | ||
(include === "active" && !oq.hasObservers()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The include: "active"
shorthand still excludes observerless queries, as before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great @benjamn - thanks!
6771746
to
a2d121f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Had a PR review inflight but merged (this is what I had) feel free to ignore.
include: ["A", abQuery], | ||
|
||
onQueryUpdated(obs, diff) { | ||
if (obs === aObs) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A mock function call to assert how many times onQueryUpdated()
is called would help us out in the future.
]); | ||
|
||
subs.push(abObs.subscribe({ | ||
next(result) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same thing about mock functions.
This PR is a follow-up to PR #8825, which was first released in Apollo Client v3.4.14. As a number of commenters have reported in issue #5419, it appears we did not completely fix the problem, so this PR attempts to finish that work. Since ObservableQuery redelivers the most recent result/error to new subscribers, it is not completely pointless to refetch an ObservableQuery that (currently) has no subscribers. However, putting the ObservableQuery in "standby" mode by setting the obsQuery.options.fetchPolicy to "standby" should still exclude the ObservableQuery from "active" status as far as refetchQueries is concerned. This logic handles useLazyQuery queries that have not yet been executed, since they start out in "standby" mode until called.
The example code I provided in #5419 (comment) should work even if
QueryIWantToRefresh
currently has no observers:I suspect the
oq.hasObservers()
check removed by this PR explains at least some instances of issue #5419, but I also believe there's value in refetching observerless queries (at the developer's explicit request), so future subscribers can immediately receive the most recent result or error.Alternatively, when performing a mutation, this same line of reasoning applies to the
refetchQueries: [...]
option:If you really want to put an
ObservableQuery
out of commission (except wheninclude: "all"
is used), you can set its.options.fetchPolicy
to"standby"
.